-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add pthread mutex deadlock detection for debug builds #24607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a57b588
to
0e10a5e
Compare
0e10a5e
to
efb88c5
Compare
efb88c5
to
a8d3d3f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nits optional)
a8d3d3f
to
e59d085
Compare
I tried out this change and it does cause an assert in |
c06ae77
to
cf6ce4a
Compare
2b24c68
to
264052a
Compare
This would really have helped in at least detecting the deadlock that was found in emscripten-core#24570 / emscripten-core#24565
264052a
to
42fb00c
Compare
// We can get here for normal mutexes too, but only in debug builds | ||
// (where we track ownership purely for debug purposed). | ||
if ((type & 15) == PTHREAD_MUTEX_NORMAL) return 0; | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we get here? Is it because of one of the changes in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, prior to this change normal mutex's never make it here since they don't need to track ownership.
Co-authored-by: Alon Zakai <alonzakai@gmail.com>
This would really have helped in at least detecting the deadlock that was found in #24570 / #24565